Skip to content

cabal-install: guard the base >= 4.22 constraint on wired-in unit ids - #12301

Merged
mergify[bot] merged 1 commit into
haskell:masterfrom
andreabedini:andrea/wip/prep/base-422-wired-in-guard
Sep 12, 2026
Merged

cabal-install: guard the base >= 4.22 constraint on wired-in unit ids#12301
mergify[bot] merged 1 commit into
haskell:masterfrom
andreabedini:andrea/wip/prep/base-422-wired-in-guard

Conversation

@andreabedini

@andreabedini andreabedini commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Fixes #12328.

dependOnWiredIns adds two kinds of constraint: one installed-unit-id
constraint per wired-in unit of the compiler, and one base >= 4.22 version
constraint added in #12055.

The second only makes sense next to the first. The unit-id constraints pin the
new, reinstallable base; the version constraint then excludes the old,
non-reinstallable one. But it is added unconditionally, while the unit-id
constraints are empty for a compiler that reports no wired-in units. Such a
compiler is affected because --allow-boot-library-installs alone is enough to
reach this code.

This makes dependOnWiredIns a no-op for a compiler that reports no wired-in
unit ids, so the version constraint applies only where the unit-id constraints
do. Behaviour is unchanged for a compiler that reports wired-in unit ids.

Test

UnitTests.Distribution.Solver.Modular.Solver gets one test in the group
"Non-reinstallable base, template-haskell and ghc (GHC without wiredInUnitIds)".
It uses the existing dbBaseOld database, which holds base-1 alone, enables
--allow-boot-library-installs and expects a successful plan. Without the fix
the test fails.

QA notes

The script in #12328 exits non-zero on an unpatched cabal. Against this branch
it succeeds with GHC 9.10.3, 9.12.2 and 9.14.1.

Checklist

Template A: This PR modifies behaviour or interface

Copilot AI lite review requested due to automatic review settings August 31, 2026 07:12
@andreabedini
andreabedini force-pushed the andrea/wip/prep/base-422-wired-in-guard branch from 09922d4 to 7adbb91 Compare August 31, 2026 07:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes an unsound solver constraint interaction in cabal-install when --allow-boot-library-installs is enabled on compilers that do not report wired-in unit ids. It ensures the base >= 4.22 constraint introduced for reinstallable base is only applied when the corresponding wired-in unit-id pinning constraints are also in effect, and adds a regression test.

Changes:

  • Guard the base >= 4.22 constraint in dependOnWiredIns on isJust (compilerInfoWiredInUnitIds compiler) so it is only applied when wired-in unit-id constraints are available.
  • Add a unit test covering the “GHC without wiredInUnitIds + allow-boot-library-installs + old base” scenario and asserting the plan is solvable.
  • Add a changelog entry documenting the fix and its motivation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
cabal-install/src/Distribution/Client/Dependency.hs Guards the base >= 4.22 constraint so it’s only added when wired-in unit-id constraints are present.
cabal-install/tests/UnitTests/Distribution/Solver/Modular/Solver.hs Adds a regression test ensuring old base plans remain solvable when no wired-in unit ids are reported.
changelog.d/base-422-wired-in-guard.md Documents the solver fix and why the guard is necessary.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@andreabedini
andreabedini requested a review from TeofilC August 31, 2026 07:14
@andreabedini
andreabedini force-pushed the andrea/wip/prep/base-422-wired-in-guard branch from 7adbb91 to 21b4724 Compare August 31, 2026 07:15
@ffaf1

ffaf1 commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

QA notes

Those are not QA notes. QA notes are meant to add a human perspective in testing, challenging the assumption of the contributor. “Testsuite goes red without the patch” is good and expected, but not useful for QA.

@andreabedini

andreabedini commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

Those are not QA notes. QA notes are meant to add a human perspective in testing, challenging the assumption of the contributor. “Testsuite goes red without the patch” is good and expected, but not useful for QA.

@ffaf1 Apologies, removed.

PS: To be honest, CONTRIBUTING.md does not make very clear what the are meant to be.

Comment thread changelog.d/base-422-wired-in-guard.md Outdated
Comment thread cabal-install/src/Distribution/Client/Dependency.hs
Comment thread cabal-install/src/Distribution/Client/Dependency.hs Outdated
Comment thread changelog.d/base-422-wired-in-guard.md Outdated
@TeofilC

TeofilC commented Aug 31, 2026

Copy link
Copy Markdown
Member

Could you also try to edit the PR description and commit message. Right now they are quite verbose and it's difficult for me to get the signal from the noise.

Also it would be helpful to create an issue.

@andreabedini

andreabedini commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

@TeofilC

All we really need to say is that we've fixed a bug in allow-boot-library-installs which caused false solver failures when used with ghc>=9.14

This is backward, we fix a bug where using --allow-boot-library-installs fails with any ghc<9.14.

We have two cases:

if isJust (compilerInfoWiredInUnitIds comp) || asBool (depResolverAllowBootLibInstalls params)
then dependOnWiredIns comp params
else dontInstallNonReinstallablePackages params

Case 1: compilerInfoWiredInUnitIds is Just _ OR allow-boot-library-installs is true.
In this case we use the new dependOnWiredIns policy.

Case 2: compilerInfoWiredInUnitIds is Nothing AND allow-boot-library-installs is false.
In this case we use the old dontInstallNonReinstallablePackages policy.

The issue is that dependOnWiredIns hardcodes base >= 4.22 regardless of the compiler version, since allow-boot-library-installs can be used with any version of the compiler.

Therefore, someone using GHC 9.12.2 with --allow-boot-library-installs won't be able to use the installed base-4.21.0.0 (because of the constraint) and won't be able to use base-4.22.0.0 either (because it fails).

I will rewrite the description. @TeofilC Do you want me to open an issue?

@TeofilC

TeofilC commented Aug 31, 2026

Copy link
Copy Markdown
Member

fails with any ghc<9.14.

Indeed it's < rather than >=.

I will rewrite the description. @TeofilC Do you want me to open an issue?

Yes please. It's always helpful to have issues for history

@andreabedini

Copy link
Copy Markdown
Collaborator Author

@TeofilC I created #12328 and rewritten the PR and the commit description.

@andreabedini
andreabedini force-pushed the andrea/wip/prep/base-422-wired-in-guard branch from b3b8da7 to e9d409a Compare September 7, 2026 06:42

@TeofilC TeofilC left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@andreabedini
andreabedini requested a review from ffaf1 September 8, 2026 09:32
Comment thread cabal-install/src/Distribution/Client/Dependency.hs Outdated
@haskell haskell deleted a comment from humh25 Sep 8, 2026
`dependOnWiredIns` adds two kinds of constraint: one installed-unit-id
constraint per wired-in unit of the compiler, and one `base >= 4.22`
version constraint.

The second only makes sense next to the first. The unit-id constraints
pin the new, reinstallable `base`; the version constraint then excludes
the old, non-reinstallable one. But it was added unconditionally, while
the unit-id constraints are empty for a compiler that reports no wired-in
units. Such a compiler has no installed `base` satisfying the bound, so
plans that reach this code fail, and `--allow-boot-library-installs`
alone is enough to reach it.

Fixes haskell#12328.
@andreabedini
andreabedini force-pushed the andrea/wip/prep/base-422-wired-in-guard branch from e9d409a to 3ff1bf0 Compare September 9, 2026 03:55
@andreabedini andreabedini added the merge me Tell Mergify Bot to merge label Sep 9, 2026
@mergify mergify Bot added ready and waiting Mergify is waiting out the cooldown period merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days labels Sep 9, 2026
@mergify mergify Bot added the queued label Sep 12, 2026
@mergify

mergify Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-09-12 14:37 UTC · Rule: default · triggered by rule Put pull requests in the rebase+merge queue
  • Checks passed · on draft merge queue: checking #12301 on master (bbed2f7) #12355
  • Merged2026-09-12 16:24 UTC · at f8737ada628ba5c387a9c42f91aa5c02f3345b57 · merge

This pull request spent 1 hour 46 minutes 3 seconds in the queue, including 1 hour 34 minutes 48 seconds running CI.

Required conditions to merge
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • github-review-approved [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Doctest Cabal
    • check-neutral = Doctest Cabal
    • check-skipped = Doctest Cabal
  • any of [🛡 GitHub branch protection]:
    • check-success = Meta checks
    • check-neutral = Meta checks
    • check-skipped = Meta checks
  • any of [🛡 GitHub branch protection]:
    • check-success = docs/readthedocs.org:cabal
    • check-neutral = docs/readthedocs.org:cabal
    • check-skipped = docs/readthedocs.org:cabal
  • any of [🛡 GitHub branch protection]:
    • check-success = Validate post job
    • check-neutral = Validate post job
    • check-skipped = Validate post job
  • any of [🛡 GitHub branch protection]:
    • check-success = fourmolu
    • check-neutral = fourmolu
    • check-skipped = fourmolu
  • any of [🛡 GitHub branch protection]:
    • check-success = hlint
    • check-neutral = hlint
    • check-skipped = hlint
  • any of [🛡 GitHub branch protection]:
    • check-success = Bootstrap post job
    • check-neutral = Bootstrap post job
    • check-skipped = Bootstrap post job
  • any of [🛡 GitHub branch protection]:
    • check-success = whitespace
    • check-neutral = whitespace
    • check-skipped = whitespace
  • any of [🛡 GitHub branch protection]:
    • check-success = Check sdist post job
    • check-neutral = Check sdist post job
    • check-skipped = Check sdist post job
  • any of [🛡 GitHub branch protection]:
    • check-success = Changelogs
    • check-neutral = Changelogs
    • check-skipped = Changelogs

@mergify
mergify Bot merged commit f8737ad into haskell:master Sep 12, 2026
69 checks passed
@mergify mergify Bot removed the queued label Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge ready and waiting Mergify is waiting out the cooldown period

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--allow-boot-library-installs fails on GHC < 9.14 with "unbuildable library from ghc-internal"

5 participants